Repository navigation
Conversation
|
wborn
left a comment
wborn
left a comment
There was a problem hiding this comment.
This is an initial AI-assisted review.
The query isolation fix looks good overall. The copy constructor preserves the caller's query options and nested predicates, and the regression tests cover reuse across different rule engine scopes.
One potential compatibility edge case is worth considering: copying an ArrayPredicate can reject JSON-serializable values that don't implement Serializable. This does not appear to affect existing usages in the repository.
No blocking issues were identified.
| return result; | ||
| } | ||
| if (value instanceof Serializable serializable) return SerializationHelper.clone(serializable); | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
ArrayPredicate.value can contain JSON-serializable objects that don't implement Serializable (e.g., Java records). These were previously handled by AssetStorageService through Jackson, but now cause the query copy to fail.
Would it make sense to support these through a Jackson fallback as well?
Description
Fixes #3478. Part of #2675.
AssetsFacade.getResultsmodified caller-owned queries when applying engine scope and storage defaults. Reusing an asset-scoped query through a realm or global engine could retain the earlier asset-path restriction. Each execution now uses an independentAssetQuerycopy, preserving the caller's filters and preventing later caller edits from changing an earlier execution's query.Add a copy constructor and copying support for nested logical groups, attribute predicates, metadata, paths and mutable values. Preserve null versus empty selections, numeric precision and predicate types. Built-in value predicates are copied explicitly; custom serializable predicates use Java serialization. The facade applies its projection to the copy, so it no longer needs to restore the caller's projection afterwards.
Regression tests cover query reuse across global, realm and asset engines, storage-added defaults, nested mutation isolation, empty selections and predicate copying. Enable
manager/src/test/groovyalongside the existing test directory. Path-filter preservation remains tracked separately in #3479.Validation:
./gradlew --offline --no-daemon :model:test :manager:test— all 393 tests passed (57 model, 336 manager). Regression tests were added and confirmed failing before the fix.Checklist